perf(memtrack): reduce serialization bottleneck by not-matthias · Pull Request #436 · CodSpeedHQ/codspeed

not-matthias · 2026-07-06T15:04:34Z

Summary

replace derived flattened MemtrackEvent serialization with a byte-identical manual serializer
make MemtrackWriter::finish return encoded frame bytes for batching
encode contiguous memtrack event batches on worker threads and write sequence-numbered zstd frames in order

Verification

cargo test -p runner-shared artifacts::memtrack
cargo test -p runner-shared
cargo check -p runner-shared
cargo test --manifest-path crates/memtrack/Cargo.toml via Nix shell with libclang/build deps; eBPF integration cases compiled but were ignored because GITHUB_ACTIONS was unset
cargo bench -p runner-shared --bench memtrack_writer

Notes

memtrack_writer bench measures the Phase A single-writer encoder path only, not the full parallel memtrack pipeline.
End-to-end memory-mode speedup was not measured in this environment.

codspeed-hq · 2026-07-06T15:10:46Z

Merging this PR will improve performance by 17.3%

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 4 improved benchmarks
✅ 3 untouched benchmarks
🆕 2 new benchmarks

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Simulation	`write_events[1000000]`	2.1 s	1.8 s	+18.23%
⚡	Simulation	`write_events[10000]`	20 ms	17 ms	+17.65%
⚡	Simulation	`write_events[100000]`	207.3 ms	177.6 ms	+16.72%
⚡	Simulation	`write_events[500000]`	1,036.6 ms	889 ms	+16.61%
🆕	Simulation	`encode_pipeline[100000]`	N/A	175.6 ms	N/A
🆕	Simulation	`encode_pipeline[1000000]`	N/A	1.8 s	N/A

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing cod-3071-fix-serialization-bottleneck (92a19a6) with main (7ce2c98)}

greptile-apps · 2026-07-06T15:16:43Z

Greptile Summary

This PR significantly restructures the memtrack encoding pipeline for performance: the old single-writer-thread design (one event per channel message, BufWriter in the wrong position relative to zstd) is replaced with a batched parallel pipeline that serializes and compresses batches concurrently on n_workers threads and reorders completed frames back into sequence before writing.

Batched ring-buffer drain: BatchAccumulator in the poll thread coalesces events into Vec<Event> chunks (up to 64 K) before crossing the channel, replacing the prior per-event mpsc channel.
Parallel encode pipeline (encode_batches): a bounded work channel distributes sequence-numbered batches to worker threads; a BTreeMap-based reorder buffer in a dedicated writer thread reassembles frames in order and writes them to the output.
Serialization fix: BufWriter now wraps zstd::Encoder (previously the reverse), coalescing rmp_serde's ~13 tiny writes per event before they reach the compressor; the manual Serialize impl for MemtrackEvent avoids the #[serde(flatten)] overhead.

Confidence Score: 4/5

Safe to merge for production use; the integration test helper has a bug that will cause all tests tracking processes with fewer than 64K allocation events to collect zero events.

The production pipeline in main.rs correctly calls stop_polling() before joining the encode thread, so real runs are unaffected. However, tests/shared.rs never calls stop_polling() — the collection loop exits on a 10-second recv_timeout rather than on channel close, so the final partial batch (which IS the only batch for any process generating fewer than 64 K events) is flushed into an unread channel after the loop has already exited. Integration tests relying on this helper will silently capture zero events.

crates/memtrack/tests/shared.rs — track_command needs to call tracker.stop_polling() after the child exits and then drain rx to channel close rather than using recv_timeout as the exit signal.

Important Files Changed

Filename	Overview
crates/memtrack/tests/shared.rs	Collection loop exits on recv_timeout rather than channel close; stop_polling() is never called, so the final partial batch (all events when total < 64K) is silently dropped for every integration test.
crates/runner-shared/src/artifacts/memtrack/pipeline.rs	New parallel encode pipeline with sequence-numbered frames, BTreeMap reordering, and bounded work channel for backpressure. Error propagation is correct: worker failures surface via join, IO errors from the writer thread surface via join order (workers → writer).
crates/runner-shared/src/artifacts/memtrack/writer.rs	BufWriter now wraps zstd::Encoder (previously the reverse), coalescing rmp_serde's ~13 tiny writes per event before they reach the compressor. finish() now returns the inner W for per-batch frame capture.
crates/runner-shared/src/artifacts/memtrack/mod.rs	Manual Serialize impl replaces #[derive(Serialize)] on MemtrackEvent to avoid flatten overhead. Byte-identity with the derived impl is verified by the new test. Deserialize is still derived; the asymmetry is intentional and correct.
crates/memtrack/src/ebpf/poller.rs	Replaced per-event mpsc channel with a BatchAccumulator behind Arc. Mutex is uncontended (single poll thread), Drop impl correctly calls shutdown(), and the straggler drain (consume + sleep + consume + flush) is preserved.
crates/memtrack/src/main.rs	track_command correctly follows the pattern: spawn child → start pipeline thread → wait for child → stop_polling → join pipeline. Error from the pipeline is properly propagated.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant KernelRingBuf
    participant PollThread
    participant BatchAccum as BatchAccumulator
    participant BatchChannel as batch_rx (crossbeam)
    participant Dispatcher as encode_batches (caller thread)
    participant WorkChannel as work_tx (bounded)
    participant Workers as Worker Threads (N)
    participant FrameChannel as frame_rx (unbounded)
    participant WriterThread as Writer Thread

    KernelRingBuf->>PollThread: ring buffer events
    PollThread->>BatchAccum: push(event)
    Note over BatchAccum: flush when len >= 64K
    BatchAccum->>BatchChannel: "send(Vec<Event>)"

    BatchChannel->>Dispatcher: IntoIterator yields batch
    Dispatcher->>WorkChannel: send((seq, batch))
    WorkChannel->>Workers: recv() encode batch
    Workers->>FrameChannel: send((seq, frame))

    FrameChannel->>WriterThread: recv()
    Note over WriterThread: BTreeMap reorder by seq
    WriterThread->>WriterThread: write_all(frame) in order

    Note over PollThread: on stop_polling():
    BatchAccum->>BatchChannel: flush() final partial batch
    PollThread-->>BatchChannel: channel closed
    BatchChannel-->>Dispatcher: IntoIterator exhausted
    Dispatcher-->>WorkChannel: drop(work_tx)
    WorkChannel-->>Workers: channel closed, exit
    Workers-->>FrameChannel: all frame_tx dropped
    FrameChannel-->>WriterThread: channel closed, exit
    WriterThread->>WriterThread: out.flush()

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant KernelRingBuf
    participant PollThread
    participant BatchAccum as BatchAccumulator
    participant BatchChannel as batch_rx (crossbeam)
    participant Dispatcher as encode_batches (caller thread)
    participant WorkChannel as work_tx (bounded)
    participant Workers as Worker Threads (N)
    participant FrameChannel as frame_rx (unbounded)
    participant WriterThread as Writer Thread

    KernelRingBuf->>PollThread: ring buffer events
    PollThread->>BatchAccum: push(event)
    Note over BatchAccum: flush when len >= 64K
    BatchAccum->>BatchChannel: "send(Vec<Event>)"

    BatchChannel->>Dispatcher: IntoIterator yields batch
    Dispatcher->>WorkChannel: send((seq, batch))
    WorkChannel->>Workers: recv() encode batch
    Workers->>FrameChannel: send((seq, frame))

    FrameChannel->>WriterThread: recv()
    Note over WriterThread: BTreeMap reorder by seq
    WriterThread->>WriterThread: write_all(frame) in order

    Note over PollThread: on stop_polling():
    BatchAccum->>BatchChannel: flush() final partial batch
    PollThread-->>BatchChannel: channel closed
    BatchChannel-->>Dispatcher: IntoIterator exhausted
    Dispatcher-->>WorkChannel: drop(work_tx)
    WorkChannel-->>Workers: channel closed, exit
    Workers-->>FrameChannel: all frame_tx dropped
    FrameChannel-->>WriterThread: channel closed, exit
    WriterThread->>WriterThread: out.flush()

Comments Outside Diff (1)

crates/memtrack/tests/shared.rs, line 149-157 (link)

Final partial batch is silently dropped for every integration test

recv_timeout exits by timeout, not by channel close. stop_polling() is never called before or during the loop, so the BatchAccumulator's partial tail batch is never flushed: flush() only runs when the poll thread shuts down, which only happens when the tracker is dropped — in a background thread that is spawned after the loop already exits. Any test whose tracked process generates fewer than DRAIN_BATCH_EVENTS (64 K) allocation events will collect 0 events because no complete batch is ever sent to the channel during the loop.

The fix mirrors what track_command in main.rs does: wait for the child to exit, call stop_polling() (which flushes the tail batch and closes the channel), then drain rx to completion via the channel-disconnect signal rather than a timeout.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/memtrack/tests/shared.rs
Line: 149-157

Comment:
**Final partial batch is silently dropped for every integration test**

`recv_timeout` exits by timeout, not by channel close. `stop_polling()` is never called before or during the loop, so the `BatchAccumulator`'s partial tail batch is never flushed: `flush()` only runs when the poll thread shuts down, which only happens when the tracker is dropped — in a background thread that is spawned *after* the loop already exits. Any test whose tracked process generates fewer than `DRAIN_BATCH_EVENTS` (64 K) allocation events will collect 0 events because no complete batch is ever sent to the channel during the loop.

The fix mirrors what `track_command` in `main.rs` does: wait for the child to exit, call `stop_polling()` (which flushes the tail batch and closes the channel), then drain `rx` to completion via the channel-disconnect signal rather than a timeout.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/memtrack/tests/shared.rs:149-157
**Final partial batch is silently dropped for every integration test**

`recv_timeout` exits by timeout, not by channel close. `stop_polling()` is never called before or during the loop, so the `BatchAccumulator`'s partial tail batch is never flushed: `flush()` only runs when the poll thread shuts down, which only happens when the tracker is dropped — in a background thread that is spawned *after* the loop already exits. Any test whose tracked process generates fewer than `DRAIN_BATCH_EVENTS` (64 K) allocation events will collect 0 events because no complete batch is ever sent to the channel during the loop.

The fix mirrors what `track_command` in `main.rs` does: wait for the child to exit, call `stop_polling()` (which flushes the tail batch and closes the channel), then drain `rx` to completion via the channel-disconnect signal rather than a timeout.

_{Reviews (3): Last reviewed commit: "refactor(memtrack): extract encode pipel..." | Re-trigger Greptile}

The custom Serialize impl emits ~13 tiny writes per event (map header + per-field key/value). With the BufWriter on the compressed output side, each of those hit zstd's streaming compressor directly, whose per-call overhead dominated: ~2.4x slower than the derived flatten path, which coalesced each event into one write. Move the BufWriter to the encoder's input side so the tiny writes are batched before reaching zstd. Restores baseline throughput.

The drain path fed events through three per-event std::mpsc hops (poll -> keepalive -> drain -> dispatcher) before batching, each send allocating a node and locking. Batch events in the poll callback instead: the single poll thread coalesces into fixed-size batches and hands off one Vec at a time. The poller now lives in the Tracker (removing the keepalive thread) and track() returns batches; main's drain thread is gone and the dispatcher just tags ordered batches with a sequence number. Per-event work drops from three mpsc sends to one Vec push.

The drain->encode->write pipeline lived inline in main.rs, so it was reachable only through the eBPF path and had no test or benchmark coverage. Move it into runner_shared::artifacts::memtrack::encode_batches, which takes any ordered batch source (the live tracker channel or synthetic data) and an output writer. Split the memtrack artifact module into mod/writer/pipeline. main.rs now spawns one thread running encode_batches. Adds unit tests for ordering across parallel workers and the empty-run frame, plus an encode_pipeline divan benchmark writing to a sink.

not-matthias added 3 commits July 6, 2026 17:01

perf(memtrack): avoid buffered serde event serialization

036742d

perf(memtrack): return encoded frames from writer

52336f2

perf(memtrack): encode event batches in parallel

9be9b22

not-matthias force-pushed the cod-3071-fix-serialization-bottleneck branch from 381bac8 to 9be9b22 Compare July 6, 2026 15:10

greptile-apps Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread crates/memtrack/src/main.rs Outdated

Comment thread crates/memtrack/Cargo.toml

Comment thread crates/memtrack/src/main.rs Outdated

not-matthias added 3 commits July 6, 2026 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf(memtrack): reduce serialization bottleneck#436

perf(memtrack): reduce serialization bottleneck#436
not-matthias wants to merge 6 commits into
mainfrom
cod-3071-fix-serialization-bottleneck

not-matthias commented Jul 6, 2026

Uh oh!

codspeed-hq Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 6, 2026 •

edited

Loading

Sequence Diagram

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

not-matthias commented Jul 6, 2026

Summary

Verification

Notes

Uh oh!

codspeed-hq Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by 17.3%

Performance Changes

Uh oh!

greptile-apps Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codspeed-hq Bot commented Jul 6, 2026 •

edited

Loading

greptile-apps Bot commented Jul 6, 2026 •

edited

Loading